perf(callgrind): cut redundant debug-info address lookups on the dump path - #33
Open
codspeed-hq[bot] wants to merge 1 commit into
Conversation
… path Writing the profile at exit queries file/line information for every instruction address of every dumped basic block (~141k addresses for `python3 testdata/test.py`). Almost all of that work is redundant: consecutive addresses belong to the same source line and the same source file. - Add a one-entry range cache to search_all_loctabs(). It answers a query that lands in the last hit loctab entry (or the next one) with a couple of compares instead of walking every DebugInfo and binary searching its loctab. It is only reused within the epoch it was found in and is dropped by advance_current_DiEpoch(), so a stale DebugInfo can never be read. - Return early from VG_(get_inline_fnname) when --read-inline-info=no. No inltab is built in that case, so the answer is always "no inlined function", but the loctab search was still performed for every dumped address. This mirrors the guard already present in VG_(new_IIPC). - Memoize the last file_node in get_debug_pos(). The existing debug cache is keyed by instruction address so it never hits while walking the distinct addresses of a basic block; every miss called CLG_(get_file_node), which rebuilds and rehashes the absolute source path. The memo compares the interned (dir, file) pointers and is reset by init_debug_cache() at the start of each dump. Callgrind output is byte-for-byte identical to the previous build.
Author
Merging this PR will not alter performance
Comparing 
Footnotes
|
Greptile SummaryThe PR reduces Callgrind dump overhead by caching adjacent debug-location lookups, skipping inline lookup when inline information is disabled, and reusing the most recently resolved source-file node.
Confidence Score: 5/5The PR appears safe to merge, with the new caches preserving the existing debug-location and file-node lookup semantics. Debug-info cache reuse is constrained by epoch invalidation and canonical loctab ranges, while the Callgrind memo uses conservative pointer identity and is reset for every dump; no concrete blocking or non-blocking defect remains.
|
| Filename | Overview |
|---|---|
| coregrind/m_debuginfo/debuginfo.c | Adds an epoch-invalidated adjacent-entry loctab cache and bypasses inline-name resolution when inline data was not read; no actionable correctness issue was found. |
| callgrind/dump.c | Adds a per-dump file-node memo keyed by object and interned source-path pointers; the key and reset boundaries preserve existing attribution behavior. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Address lookup] --> B{Inline lookup disabled?}
B -->|Yes| C[Return no inline name]
B -->|No or file/line lookup| D{Current epoch and cached loctab entry covers address?}
D -->|Yes| E[Reuse DebugInfo and location]
D -->|Next entry covers| F[Advance cached location]
D -->|No| G[Walk DebugInfos and search loctab]
G --> H[Refresh epoch-scoped cache]
E --> I{Same object and interned dir/file?}
F --> I
H --> I
I -->|Yes| J[Reuse file_node]
I -->|No| K[Resolve and cache file_node]
Reviews (1): Last reviewed commit: "perf(callgrind): cut redundant debug-inf..." | Re-trigger Greptile
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Writing the profile at exit is a significant part of a Callgrind run: on the
python3 testdata/test.pyworkload the dump phase queries file/line information for ~141k instruction addresses. Almost all of that work is redundant — consecutive addresses belong to the same source line and the same source file.Three changes remove that redundancy:
1. One-entry range cache for
search_all_loctabs()(coregrind/m_debuginfo/debuginfo.c)Every
VG_(get_filename_linenum)/VG_(get_inline_fnname)call walks the wholeDebugInfolist and then binary-searches that object'sloctab. Since one source line covers several instructions, consecutive queries land in the same loctab entry (or the next one). The cache remembers the last hit and answers those queries with a couple of compares.It is only reused for the epoch it was found in, and it is dropped in
advance_current_DiEpoch()— i.e. whenever debug info is loaded or discarded — so a staleDebugInfocan never be read.2. Skip the loctab search in
VG_(get_inline_fnname)when inline info was never readCallgrind defaults to
--read-inline-info=no, in which case noinltabis ever built and the answer is always "no inline function" — but the search was still performed for every dumped address. The guard mirrors the one already present in its siblingVG_(new_IIPC).3. Reuse the last
file_nodeinget_debug_pos()(callgrind/dump.c)The existing debug cache is keyed by instruction address, so it never hits while a dump walks the (all distinct) addresses of a basic block. Every miss called
CLG_(get_file_node), which rebuilds the absolute source path on the stack and hashes it character by character. The memo compares the interned(dir, file)pointers returned by the debug info and reuses the previous node; it is reset byinit_debug_cache()at the start of each dump, and no guest code runs while dumping, so no debug info can change in between.Correctness
Verified locally (amd64-linux,
--enable-only64bit, Capstone enabled):/bin/echo hellowith--read-inline-info=yes(exercising thecfni=inline path) and on/bin/ls /etcwith--separate-callers=3— the fullcallgrind.outincluding allfl=/fi=/fn=numbering and event counts. Both builds were produced from the same directory and run with ASLR disabled and an identical environment, so the comparison is exact.search_all_loctabs.make checkcompletes successfully.Measurement
Measured with CodSpeed walltime runs in a sandbox (x86_64, not a macro runner). Both builds were placed on the same filesystem and interleaved inside a single CodSpeed run (12 rounds each, 1s warmup), so base and head were measured under identical machine conditions — a first attempt using two sequential runs was unusable because the whole machine drifted ~2-3% between them.
echo, no-inlineecho, inlineecho, full-no-inlineecho, full-with-inlinepython3 test.py, no-inlinepython3 test.py, inlinepython3 test.py, full-no-inlinepython3 test.py, full-with-inlineAll 8 configurations improve; the win is largest where the dump is a meaningful share of the run and where the
--read-inline-info=noearly return applies. Execution-dominated workloads are unaffected, as expected. The definitive numbers will come from the repository's own CodSpeed job on the macro runner.Interleaved A/B run: https://app.codspeed.io/CodSpeedHQ/valgrind-codspeed/runs/6a6db8f71be703b08d0d17ba